-
Notifications
You must be signed in to change notification settings - Fork 163
Fix error when a shared event loop is unset #1180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1180 +/- ##
=======================================
Coverage 90.53% 90.53%
=======================================
Files 2 2
Lines 412 412
Branches 45 45
=======================================
Hits 373 373
Misses 30 30
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5ff08a0
to
bb8628a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e9b9fb9
to
ac9e0d6
Compare
@seifertm I've pushed the patch that satisfies the edge-case you mentioned (though the PR is still quite proof-of-concept). |
ac9e0d6
to
8214743
Compare
8214743
to
f5739a6
Compare
@tjkuson I continued working on your PR and found that the issue has resolved itself as part of a recent merge. Using your tests, I did a git bisect to identify the change leading to the bug fix, but I didn't really succeed and didn't want to invest further time in it. I think the new tests are extremely helpful on its own and I'd like to merge them along with your cleanup and changelog entries. If it's not too much work, I'd like to backport the fix and do a patch release for v1.1 (given I identify the fixing commit) and the tests will help with that. |
@seifertm Thank you, that's great! Glad to hear the other stuff in the PR is helpful. |
Automatically reinstates the event loop on the main thread when needed.
The coverage report suggests the patch is uncovered, but I think this is due to limitations of measuring coverage across processes.
Fixes #1177
Note: this is a work-in-progress proposal based on my understanding of the issue. If the maintainers prefer a different approach or consider the issue invalid, I’m happy to adjust or close this PR.